[Autofill password import] Misc fixes #1184
Open
+175
−54
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
✅ Deploy Preview for content-scope-scripts ready!
To edit notification comments on pull requests, go to your Netlify site configuration. |
dbajpeyi
changed the title
Temporary Branch UpdateThe temporary branch has been updated with the latest changes. Below are the details:
Please use the above install command to update to the latest version. |
[Beta] Generated file diffTime updated: Fri, 01 Nov 2024 14:13:46 GMT AndroidFile has changed IntegrationFile has changed |
dbajpeyi
force-pushed
the
dbajpeyi/fix/supported-paths
branch
from
1058d41
to
b079299
Compare
dbajpeyi
changed the title
dbajpeyi
force-pushed
the
dbajpeyi/fix/supported-paths
branch
5 times, most recently
from
297ad13
to
11e20f0
Compare
dbajpeyi
force-pushed
the
dbajpeyi/fix/supported-paths
branch
from
11e20f0
to
b3ade5c
Compare
dbajpeyi
force-pushed
the
dbajpeyi/fix/supported-paths
branch
from
dd502ec
to
7a69964
Compare
dbajpeyi
force-pushed
the
dbajpeyi/fix/supported-paths
branch
from
65dac43
to
38f843d
Compare
…content-scope-scripts into dbajpeyi/fix/supported-paths
dbajpeyi
force-pushed
the
dbajpeyi/fix/supported-paths
branch
from
0afb993
to
100aa6b
Compare
dbajpeyi
commented
Nov 4, 2024
| @@ -41,7 +41,7 @@ export class UpdateNotificationService { | |||
| */ | |||
| dismiss () { | |||
| this.ntp.messaging.notify('updateNotification_dismiss') | |||
| // eslint-disable-next-line @typescript-eslint/no-unused-vars | |||
There was a problem hiding this comment.
Some new eslint changes seem to have forced this when I did a lint-fix
…content-scope-scripts into dbajpeyi/fix/supported-paths
dbajpeyi
force-pushed
the
dbajpeyi/fix/supported-paths
branch
from
7964075
to
8ad828f
Compare
dbajpeyi
force-pushed
the
dbajpeyi/fix/supported-paths
branch
from
6caf7d9
to
0ecadc6
Compare
dbajpeyi
force-pushed
the
dbajpeyi/fix/supported-paths
branch
from
0ecadc6
to
39d938b
Compare
| if (shouldTap) { | ||
| this.autotapElement(element) | ||
| } else { | ||
| setTimeout(() => this.animateElement(element, style), 300) |
There was a problem hiding this comment.
Can you make this 300 a constant, mainly to document what the wait is for a little bit (comment is probably unnecessary with a well named var).
| const { top, left, width, height } = elementToCenterOn.getBoundingClientRect() | ||
| overlay.style.position = 'absolute' | ||
|
|
||
| overlay.style.top = `calc(${top}px + ${window.scrollY}px - ${isRound ? height / 2 : 0}px - 1px - ${style.offsetTopEm}em)` |
There was a problem hiding this comment.
I'd prefer these ternaries as variables outside of the template string to simplify the expression a little. Feel free to skip if you care about keeping.
| if (shouldTap) { | ||
| this.autotapElement(element) | ||
| } else { | ||
| setTimeout(() => this.animateElement(element, style), ANIMATION_TIMEOUT) |
There was a problem hiding this comment.
Maybe this is safer:
const domLoaded = new Promise((resolve) => {
if (document.readyState === "loading") {
document.addEventListener("DOMContentLoaded", resolve);
} else {
resolve();
}
})
Suggested change
| setTimeout(() => this.animateElement(element, style), ANIMATION_TIMEOUT) | |
| await domLoaded | |
| this.animateElement(element, style) |
dbajpeyi
force-pushed
the
dbajpeyi/fix/supported-paths
branch
from
e4d9a72
to
7d4a3af
Compare
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Asana Task/Github Issue: https://app.asana.com/0/1205996472158114/1208671198302487/f
Description
indexOfbyincludes, however I replaced the functionality 😄Original PR: [Google password manager] Android isolated module for password import #1157. This was causing login to break, and also causing other animations to break.
scrollIntoViewfails if the layout is not fully rendered (edge case).Testing Steps
feature/craig/autofill_gpm_ui_to_launch_from_password_manager) with the confighttps://jsonblob.com/api/1301563859532636160Checklist
Please tick all that apply: